lowering: extend temporary lifetimes around await #64292

davidtwco · 2019-09-08T20:27:18Z

Centril

Could you please also update the doc comment on fn lower_expr_await to accurately reflect the new desugaring and then follow up with a PR against the reference (See rust-lang/reference#635 for @nikomatsakis's initial PR.)?

src/test/ui/async-await/issue-63832-await-short-temporary-lifetime-1.rs

Signed-off-by: David Wood <david@davidtw.co>

davidtwco · 2019-09-08T22:27:36Z

Thanks @Centril, fixed those comments and opened rust-lang/reference#674.

src/librustc/hir/lowering/expr.rs

This commit changes the HIR lowering around `await` so that temporary lifetimes are extended. Previously, await was lowered as: ```rust { let mut pinned = future; loop { match ::std::future::poll_with_tls_context(unsafe { <::std::pin::Pin>::new_unchecked(&mut pinned) }) { ::std::task::Poll::Ready(result) => break result, ::std::task::Poll::Pending => {} } yield (); } } ``` With this commit, await is lowered as: ```rust match future { mut pinned => loop { match ::std::future::poll_with_tls_context(unsafe { <::std::pin::Pin>::new_unchecked(&mut pinned) }) { ::std::task::Poll::Ready(result) => break result, ::std::task::Poll::Pending => {} } yield (); } } ``` However, this change has the following side-effects: - All temporaries in future will be considered to live across a yield for the purpose of auto-traits. - Borrowed temporaries in future are likely to be considered to be live across the yield for the purpose of the generator transform. Signed-off-by: David Wood <david@davidtw.co>

matthewjasper · 2019-09-10T11:18:54Z

@bors r+

bors · 2019-09-10T11:18:55Z

📌 Commit 63fad69 has been approved by matthewjasper

@matthewjasper

…ry-lifetimes, r=matthewjasper lowering: extend temporary lifetimes around await Fixes rust-lang#63832. r? @matthewjasper cc @nikomatsakis

@ghost

Rollup of 8 pull requests Successful merges: - #63786 (Make `abs`, `wrapping_abs`, `overflowing_abs` const functions) - #63989 (Add Yaah to clippy toolstain notification list) - #64256 (test/c-variadic: Fix patterns on powerpc64) - #64292 (lowering: extend temporary lifetimes around await) - #64311 (lldb: avoid mixing "Hit breakpoint" message with other output.) - #64330 (Clarify E0507 to note Fn/FnMut relationship to borrowing) - #64331 (Changed instant is earlier to instant is later) - #64344 (rustc_mir: buffer -Zdump-mir output instead of pestering the kernel constantly.) Failed merges: r? @ghost

RalfJung · 2019-09-11T14:54:34Z

Is it expected that this is a breaking change? A Miri testcase is failing now:

2019-09-10T19:17:04.0564321Z normalized stderr:
2019-09-10T19:17:04.0564553Z error[E0597]: `x` does not live long enough
2019-09-10T19:17:04.0565934Z   --> $DIR/async-fn.rs:18:13
2019-09-10T19:17:04.0566063Z    |
2019-09-10T19:17:04.0566130Z 18 |     async { x + y }.await
2019-09-10T19:17:04.0566458Z    |     --------^------
2019-09-10T19:17:04.0566539Z    |     |     | |
2019-09-10T19:17:04.0566641Z    |     |     | borrowed value does not live long enough
2019-09-10T19:17:04.0566727Z    |     |     value captured here by generator
2019-09-10T19:17:04.0566873Z    |     a temporary with access to the borrow is created here ...
2019-09-10T19:17:04.0566971Z 19 | }
2019-09-10T19:17:04.0567254Z    | -
2019-09-10T19:17:04.0567323Z    | |
2019-09-10T19:17:04.0567409Z    | `x` dropped here while still borrowed
2019-09-10T19:17:04.0567524Z    | ... and the borrow might be used here, when that temporary is dropped and runs the destructor for type `impl std::future::Future`
2019-09-10T19:17:04.0567653Z    |
2019-09-10T19:17:04.0568341Z    = note: The temporary is part of an expression at the end of a block. Consider forcing this temporary to be dropped sooner, before the block's local variables are dropped. For example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block.
2019-09-10T19:17:04.0568497Z 
2019-09-10T19:17:04.0568622Z error[E0597]: `y` does not live long enough
2019-09-10T19:17:04.0568922Z   --> $DIR/async-fn.rs:18:17
2019-09-10T19:17:04.0569038Z    |
2019-09-10T19:17:04.0569103Z 18 |     async { x + y }.await
2019-09-10T19:17:04.0569731Z    |     ------------^--
2019-09-10T19:17:04.0569811Z    |     |     |     |
2019-09-10T19:17:04.0569908Z    |     |     |     borrowed value does not live long enough
2019-09-10T19:17:04.0569999Z    |     |     value captured here by generator
2019-09-10T19:17:04.0570108Z    |     a temporary with access to the borrow is created here ...
2019-09-10T19:17:04.0570189Z 19 | }
2019-09-10T19:17:04.0570464Z    | -
2019-09-10T19:17:04.0570534Z    | |
2019-09-10T19:17:04.0570619Z    | `y` dropped here while still borrowed
2019-09-10T19:17:04.0570726Z    | ... and the borrow might be used here, when that temporary is dropped and runs the destructor for type `impl std::future::Future`
2019-09-10T19:17:04.0570849Z    |
2019-09-10T19:17:04.0571489Z    = note: The temporary is part of an expression at the end of a block. Consider forcing this temporary to be dropped sooner, before the block's local variables are dropped. For example, you could save the expression's value in a new local variable `x` and then make `x` be the expression at the end of the block.
2019-09-10T19:17:04.0571685Z 
2019-09-10T19:17:04.0571753Z error: aborting due to 2 previous errors

fix async test Test got broken by rust-lang/rust#64292.

yoshuawuyts · 2019-09-11T15:25:58Z

Not sure if this was intended to cause breakage, but we had to patch our code in async-std likely because of this PR: https://github.com/async-rs/async-std/pull/182/files.

before

 peer.send(format!("from {}: {}\n", from, msg)).await?

after

let msg = format!("from {}: {}\n", from, msg);
peer.send(msg).await?

seanmonstar · 2019-09-11T22:11:23Z

So are the above examples a regression?

Centril · 2019-09-11T23:11:17Z

Depends... can you create a minimal reproducer for before/after?

tmandry · 2019-09-11T23:16:30Z

This causes errors to crop up all over the Fuchsia code base. Based on #63832 (comment), it seems like this might have been expected, however.

seanmonstar · 2019-09-11T23:59:34Z

@Centril The change in miri is quite succinct, and the "before" doesn't appear to be bad... while the "after" seems odd? https://github.com/rust-lang/miri/pull/946/files#diff-ecc42d2008d431ab52ee698995abd41b

Is something more needed?

Inlined:

Before:

async fn add(x: u32, y: u32) -> u32 {
    async { x + y }.await
}

After:

async fn add(x: u32, y: u32) -> u32 {
    let a = async { x + y };
    a.await
}

davidtwco · 2019-09-12T08:50:28Z

I've confirmed locally that this patch did cause these regressions. I'm not sure what the best way to proceed here is, @nikomatsakis @Centril @matthewjasper?

pimeys · 2019-09-12T14:11:40Z

Seems that this patch broke my test server with Tide.

https://github.com/pimeys/blocking_test/

Works with 2019-09-10, broken in 2019-09-11.

- 771c5d1 Add macros in extern blocks and new proc-macro support. - 8caabd6 Update for "modern" `meta` matcher. - 1b44947 Update await desugaring after #64292

Update reference - Add macros in extern blocks and new proc-macro support. - Update for "modern" `meta` matcher. - Update await desugaring after rust-lang#64292

Signed-off-by: David Wood <david@davidtw.co>

rust-highfive assigned matthewjasper Sep 8, 2019

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Sep 8, 2019

davidtwco mentioned this pull request Sep 8, 2019

temporary lifetime around await is (maybe) unexpectedly short #63832

Closed

Centril reviewed Sep 8, 2019

View reviewed changes

src/test/ui/async-await/issue-63832-await-short-temporary-lifetime-1.rs Outdated Show resolved Hide resolved

Centril added A-async-await Area: Async & Await F-async_await labels Sep 8, 2019

davidtwco added a commit to davidtwco/reference that referenced this pull request Sep 8, 2019

Update await desugaring after rust-lang/rust#64292

1b44947

Signed-off-by: David Wood <david@davidtw.co>

davidtwco force-pushed the issue-63832-await-temporary-lifetimes branch from 78ea5f5 to 2f3c957 Compare September 8, 2019 22:26

davidtwco mentioned this pull request Sep 8, 2019

Update await desugaring rust-lang/reference#674

Merged

Centril approved these changes Sep 8, 2019

View reviewed changes

matthewjasper reviewed Sep 9, 2019

View reviewed changes

src/librustc/hir/lowering/expr.rs Outdated Show resolved Hide resolved

davidtwco force-pushed the issue-63832-await-temporary-lifetimes branch from 2f3c957 to 8e3fb22 Compare September 9, 2019 18:08

davidgauch reviewed Sep 10, 2019

View reviewed changes

src/librustc/hir/lowering/expr.rs Outdated Show resolved Hide resolved

davidtwco force-pushed the issue-63832-await-temporary-lifetimes branch from 8e3fb22 to 63fad69 Compare September 10, 2019 10:28

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 10, 2019

Centril mentioned this pull request Sep 10, 2019

Rollup of 8 pull requests #64354

Merged

bors merged commit 63fad69 into rust-lang:master Sep 10, 2019

jebrosen added a commit to rwf2/Rocket that referenced this pull request Sep 11, 2019

Fix a minor compilation error, possibly caused by rust-lang/rust#64292.

c7134c8

RalfJung mentioned this pull request Sep 11, 2019

fix async test rust-lang/miri#946

Merged

bors added a commit to rust-lang/miri that referenced this pull request Sep 11, 2019

Auto merge of #946 - RalfJung:rustup, r=oli-obk

7278385

fix async test Test got broken by rust-lang/rust#64292.

davidbarsky mentioned this pull request Sep 11, 2019

Tokio-tls compile error tokio-rs/tokio#1546

Closed

sfackler mentioned this pull request Sep 12, 2019

async functions fail to move values into generated futures in the latest nightly #64391

Closed

dekellum mentioned this pull request Sep 13, 2019

RFC: #[cfg(accessible(..) / version(..))] rust-lang/rfcs#2523

Merged

dtolnay mentioned this pull request Sep 15, 2019

Spurious new Send requirement in async block #64477

Closed

davidtwco deleted the issue-63832-await-temporary-lifetimes branch September 17, 2019 09:24

nikomatsakis mentioned this pull request Sep 17, 2019

Lifetime errors on nightly-2019-09-11 but not nightly-2019-09-10 #64433

Closed

jebrosen added a commit to rwf2/Rocket that referenced this pull request Sep 21, 2019

Fix a minor compilation error, possibly caused by rust-lang/rust#64292.

bdbf80f

bstrie mentioned this pull request Sep 23, 2019

Tracking issue for async/await (RFC 2394) #50547

Closed

11 tasks

This was referenced Oct 4, 2019

Lint for using await while holding a MutexGuard rust-lang/rust-clippy#4226

Closed

check_mod_item_types query times regressed bigly #65147

Closed

tmandry added a commit that referenced this pull request Oct 6, 2019

Update reference

8baf7bf

- 771c5d1 Add macros in extern blocks and new proc-macro support. - 8caabd6 Update for "modern" `meta` matcher. - 1b44947 Update await desugaring after #64292

tmandry mentioned this pull request Oct 6, 2019

Update reference #65173

Merged

jebrosen added a commit to jebrosen/Rocket that referenced this pull request Dec 11, 2019

Fix a minor compilation error, possibly caused by rust-lang/rust#64292.

270c85b

yoshuawuyts mentioned this pull request Dec 17, 2019

A problem with switching from 0.3 to 0.4 http-rs/tide#370

Closed

SergioBenitez pushed a commit to rwf2/Rocket that referenced this pull request Jul 10, 2020

Fix a minor compilation error, possibly caused by rust-lang/rust#64292.

54bba63

SergioBenitez pushed a commit to rwf2/Rocket that referenced this pull request Jul 10, 2020

Fix a minor compilation error, possibly caused by rust-lang/rust#64292.

d05d6ea

SergioBenitez pushed a commit to rwf2/Rocket that referenced this pull request Jul 11, 2020

Fix a minor compilation error, possibly caused by rust-lang/rust#64292.

aa3ff71

SergioBenitez pushed a commit to rwf2/Rocket that referenced this pull request Jul 11, 2020

Fix a minor compilation error, possibly caused by rust-lang/rust#64292.

09f2f94

SergioBenitez pushed a commit to rwf2/Rocket that referenced this pull request Jul 11, 2020

Fix a minor compilation error, possibly caused by rust-lang/rust#64292.

5763e72

SergioBenitez pushed a commit to rwf2/Rocket that referenced this pull request Jul 11, 2020

Fix a minor compilation error, possibly caused by rust-lang/rust#64292.

62a99e9

Havvy pushed a commit to Havvy/reference that referenced this pull request Aug 25, 2020

Update await desugaring after rust-lang/rust#64292

fb38125

Signed-off-by: David Wood <david@davidtw.co>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lowering: extend temporary lifetimes around await #64292

lowering: extend temporary lifetimes around await #64292

davidtwco commented Sep 8, 2019

Centril left a comment

davidtwco commented Sep 8, 2019

matthewjasper commented Sep 10, 2019

bors commented Sep 10, 2019

RalfJung commented Sep 11, 2019

yoshuawuyts commented Sep 11, 2019

seanmonstar commented Sep 11, 2019

Centril commented Sep 11, 2019

tmandry commented Sep 11, 2019

seanmonstar commented Sep 11, 2019 •

edited

Loading

davidtwco commented Sep 12, 2019

pimeys commented Sep 12, 2019

lowering: extend temporary lifetimes around await #64292

lowering: extend temporary lifetimes around await #64292

Conversation

davidtwco commented Sep 8, 2019

Centril left a comment

Choose a reason for hiding this comment

davidtwco commented Sep 8, 2019

matthewjasper commented Sep 10, 2019

bors commented Sep 10, 2019

RalfJung commented Sep 11, 2019

yoshuawuyts commented Sep 11, 2019

seanmonstar commented Sep 11, 2019

Centril commented Sep 11, 2019

tmandry commented Sep 11, 2019

seanmonstar commented Sep 11, 2019 • edited Loading

davidtwco commented Sep 12, 2019

pimeys commented Sep 12, 2019

seanmonstar commented Sep 11, 2019 •

edited

Loading